feat: Adding support for ResVPNProxy data#243
Conversation
clintjedwards
force-pushed
the
cje/resvpnproxy
branch
from
be4e93e to
6cf09b8
Compare
| result := &ResVPNProxyResult{} | ||
| var err error | ||
|
|
||
| result.IsAnonymous, err = r.callDownstreamBool(r.downstream.req.DownstreamResvpnproxyIsAnonymous) |
There was a problem hiding this comment.
We already have a helper called ignoreNoneError that you can use for this. See example usage here: https://github.com/fastly/compute-sdk-go/blob/main/fsthttp/backend.go#L111
There was a problem hiding this comment.
Removed the Downstream bool helper and used the ignoreNoneError from the main package.
| b bool | ||
| _ prim.Usize // align padding | ||
| } | ||
| status := fastlyHTTPDownstreamResvpnproxyIsAnonymous(r.h, prim.ToPointer(&result.b)) |
There was a problem hiding this comment.
Our hostcall wrappers generally look like this shorter form:
if err := fastlyBackendIsDynamic(
nameBuffer.Data, nameBuffer.Len,
prim.ToPointer(&dynamic),
).toError(); err != nil {
return false, err
}
return dynamic != 0, nil
and I think you should be able that form here.
There was a problem hiding this comment.
I've always found the ; err !=nil { pattern a bit harder to read than simply separating them out. But I've updated it for consistency.
| ) FastlyStatus | ||
|
|
||
| // ResVPNProxyLookup returns the Proxy and VPN data associated with the IP address. | ||
| func ResVPNProxyLookup(ip net.IP) ([]byte, error) { |
There was a problem hiding this comment.
This function doesn't seem to be exposed to the user at all. Should it be?
There was a problem hiding this comment.
A holdover from me attempting to first create a more generalizable function instead of one that just works off the request. I first figured we could do that and then just use it everywhere else and also offer it to the user, but then I abandoned due to time constraints.
Good catch, I'll remove.
| result prim.Pointer[bool], | ||
| ) FastlyStatus | ||
|
|
||
| func (r *HTTPRequest) DownstreamResvpnproxyIsAnonymousVpn() (bool, error) { |
| result prim.Pointer[bool], | ||
| ) FastlyStatus | ||
|
|
||
| func (r *HTTPRequest) DownstreamResvpnproxyIsProxyOverVpn() (bool, error) { |
| result prim.Pointer[bool], | ||
| ) FastlyStatus | ||
|
|
||
| func (r *HTTPRequest) DownstreamResvpnproxyIsSmartDnsProxy() (bool, error) { |
| result prim.Pointer[bool], | ||
| ) FastlyStatus | ||
|
|
||
| func (r *HTTPRequest) DownstreamResvpnproxyIsVpnDatacenter() (bool, error) { |
| nwrittenOut prim.Pointer[prim.Usize], | ||
| ) FastlyStatus | ||
|
|
||
| func (r *HTTPRequest) DownstreamResvpnproxyVpnServiceName() (string, error) { |
| // | ||
| //go:wasmimport fastly_resvpnproxy lookup | ||
| //go:noescape | ||
| func fastlyResVpnProxyLookup( |
There was a problem hiding this comment.
Other places resvpnproxy has been camel-cased as Resvpnproxy. We should be consistent.
There was a problem hiding this comment.
I grepped through and tried to correct all instances of this. Good call, its important to be consistent about these things.
|
|
||
| // ResVPNProxyResult represents additional IP Proxy and VPN Intelligence data for a request. | ||
| type ResVPNProxyResult struct { | ||
| IsAnonymous bool // True if the IP address is present in one or more categories of anonymous flags. |
There was a problem hiding this comment.
These fields should have JSON struct tags. (This also means the resvpnproxy example can be simplified to not have to cosntruct the JSON by hand.)
There was a problem hiding this comment.
Added! Working in Rust too long, I completely forgot about struct tags for this one.
clintjedwards
force-pushed
the
cje/resvpnproxy
branch
from
6cf09b8 to
1f92683
Compare
| // Test the ResVPNProxy data using the request method | ||
| vpnData, err := r.ResVPNProxyData() | ||
| if err != nil { | ||
| w.WriteHeader(http.StatusInternalServerError) |
There was a problem hiding this comment.
fsthttp has its own copy of these constants: https://github.com/fastly/compute-sdk-go/blob/main/fsthttp/status.go#L66
| ) FastlyStatus | ||
|
|
||
| func (r *HTTPRequest) DownstreamResVPNProxyIsAnonymous() (bool, error) { | ||
| var result bool |
There was a problem hiding this comment.
You lost the padding structs in all the hostcalls.
There was a problem hiding this comment.
Good catch. I wasn't 100% sure why they were necessary since testing seemed to work without them, but I assume I just got lucky with the alignment.
I've returned them and also refactored my functions to look more like the others in terms of whitespace.
dgryski
left a comment
dgryski
left a comment
There was a problem hiding this comment.
Oops, forgot I had a few changes needed: the alignment struct and the http constant.
Adding a lookup function for ResVPNProxy. This enables VPN/Proxy intelligence data for specific IP addresses.
clintjedwards
force-pushed
the
cje/resvpnproxy
branch
from
1f92683 to
2fb1c82
Compare
2 participants
Adding a lookup function for ResVPNProxy. This enables VPN/Proxy intelligence data for specific IP addresses.
Most of this code is copied from other implementations for features, I figured we could start with an extra call off of fsthttp and then if we needed later we could make a standalone package as well just like
geo.